-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Avoid casting to double type unnecessarily when setting values i… #23462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hello @icexelloss! Thanks for submitting the PR.
|
b43b803
to
ca6bbd3
Compare
Codecov Report
@@ Coverage Diff @@
## master #23462 +/- ##
=======================================
Coverage 92.23% 92.23%
=======================================
Files 161 161
Lines 51197 51197
=======================================
Hits 47220 47220
Misses 3977 3977
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comments. pls add a whatsnew note in bug fixes / indexing. ping on green.
@@ -80,3 +80,16 @@ def test_numpy_timedelta_scalar_indexing(self, start, stop, | |||
result = s.loc[slice(start, stop)] | |||
expected = s.iloc[expected_slice] | |||
tm.assert_series_equal(result, expected) | |||
|
|||
def test_set_dataframe_column_by_index(self): | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add this PR number here as a comment
@@ -80,3 +80,16 @@ def test_numpy_timedelta_scalar_indexing(self, start, stop, | |||
result = s.loc[slice(start, stop)] | |||
expected = s.iloc[expected_slice] | |||
tm.assert_series_equal(result, expected) | |||
|
|||
def test_set_dataframe_column_by_index(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can rename to
test_roundtrip_thru_setitem
|
||
df = pd.DataFrame({'dt': pd.Series([dt1, dt2])}) | ||
s = pd.Series([dt1]) | ||
value_before = df['dt'].iloc[1].value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name this expected
s = pd.Series([dt1]) | ||
value_before = df['dt'].iloc[1].value | ||
df.loc[[True, False]] = s | ||
value_after = df['dt'].iloc[1].value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name this result
value_before = df['dt'].iloc[1].value | ||
df.loc[[True, False]] = s | ||
value_after = df['dt'].iloc[1].value | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also test
tm.assert_frame_equal(df, df_copy)
(and copy df on line 89)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
ca6bbd3
to
62ab354
Compare
lgtm. ping on green. |
thanks @icexelloss |
Thanks @jreback ! |
…n time delta column
git diff upstream/master -u -- "*.py" | flake8 --diff
When setting a timedelta column in a dataframe using
pd.Series
or array as index, there is some rounding error because the underlying TimeDeltaBlock is casted to double and back to int. This patch fixes the issue.